feat: enable log forwarding through otel collector by florentianayuwono · Pull Request #186 · canonical/github-runner-operators

florentianayuwono · 2026-04-22T15:54:38Z

What this PR does

Add action to allow workflow authors to opt in to forwarding specific log files from self-hosted GitHub runners to Loki through the OpenTelemetry Collector snap.

Why we need it

So that workflow author can opt-in to forwarding specific log files from their job to Grafana, and view application logs alongside system metrics without changing their existing logging approach.

Checklist

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

…v v8 (#182) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(dashboard): remove dead override hiding job queue time series The "Job queue time" panel had a leftover hideSeriesFrom override that excluded every series except one specific named expression. Combined with the panel's current query (which already aggregates with sum by (le)), the override hid all data, leaving an empty chart. The override is obsolete now that the query collapses every label except le into a single combined histogram, so removing it restores visualisation without any other change. * ci: pin astral-sh/setup-uv to v8.1.0 The astral-sh/setup-uv repository does not publish a floating v8 major tag (only v8.0.0 and v8.1.0 specific tags exist), so referencing @v8 fails to resolve and breaks the workflow on every PR.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Christopher Bartz <christopher.bartz@canonical.com>

* chore(docs): update contributing guidelines (charmkeeper) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(docs): build errors * chore: clarify PR checklist and remove unnecessary item * chore(docs): revert changes in CONTRIBUTING.md * fix(docs): whoops we're ignoring the changelog * fix: update pr checklist to incorporate previous items, remove duplicate items --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Adds a new composite GitHub Action that can opt workflows into forwarding selected log files from self-hosted runners to Loki via an OpenTelemetry Collector snap.

Changes:

Introduces actions/enable-log-forwarding composite action that writes an otelcol config fragment and restarts the collector.
Adds unit tests and a CI workflow (including a self-hosted smoke test) for the new action.
Documents usage in a new how-to guide and records the change in the docs changelog.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
docs/how-to/enable-log-forwarding.md	New how-to page describing how to use the log-forwarding action and query hints.
docs/changelog.md	Adds a 2026-04-22 entry documenting the new action.
actions/enable-log-forwarding/tests/test_enable_log_forwarding.py	Unit tests for parsing inputs, endpoint resolution, exporter detection, and config generation.
actions/enable-log-forwarding/enable-log-forwarding.py	Action implementation that generates an otelcol config fragment, installs it, and restarts the collector.
actions/enable-log-forwarding/action.yaml	Composite action definition and inputs wiring to the Python script.
.github/workflows/enable_log_forwarding_action_tests.yaml	CI workflow to run unit tests and a self-hosted smoke test for the action.

erinecon

Thanks for adding a how-to guide with this PR! Some initial comments and questions

yanksyoon

Initial review done, thank you!

Co-authored-by: Erin Conley <erin.conley@canonical.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Erin Conley <erin.conley@canonical.com>

cbartz · 2026-04-23T07:01:41Z

Should we also update the README? Usually the front page is the one the users are looking at (at least I do) cc @erinecon

I know the current state of the README is not up to date, it still marks the repo as WIP. Any opinions @erinecon ?

Should we also update the README?

Yes we should update the README, but my question would be whether those updates fall under the scope
of this PR or if we should update the README in a follow-up PR.

I know the current state of the README is not up to date, it still marks the repo as WIP.

Is the repo still WIP? We should continue to clearly indicate that, but regardless, I still think we should update the README.

i think readme should be done in a separate pr / jira ticket

I'm fine with this, would it be helpful if I opened an issue into the repo, or would you just prefer the Jira ticket?

@erinecon yes i think we need separate jira ticket for readme (cc @cbartz)

cbartz · 2026-04-23T07:03:05Z

+
+
+def build_config(files, resolved_endpoint, exporter_already_exists):
+    attrs = [


could we use a templated string ? so that we can more quickly understand the generated config and avoid the requirement to read the python code to get a bigger picture

i added a j2 file for this :)

I think using a template for JSON is not a very good idea, as you need to consider escape characters if the input contains something like " or \n when using a template to compose a JSON file. It's much cleaner and more error-proof to use Python's json library. This also saves us from installing the jinja2 package.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <copilot@github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

yanksyoon

A few more nits, almost there! Thanks @florentianayuwono :)

yanksyoon · 2026-04-24T04:01:09Z

+MODULE_PATH = (
+    pathlib.Path(__file__).resolve().parent.parent / "enable_log_forwarding.py"
+)


Would there be a better way to load the module? I'm wondering if we can safely assume that the test will be run from project root and import using the path relative to project root.

Since load_module function for a test seems like a bit of an overkill.

yanksyoon · 2026-04-24T04:02:01Z

+MODULE = load_module(MODULE_PATH)
+
+
+class TestEnableLogForwarding(unittest.TestCase):


Could we approach our tests with pytest and in functional testing style please? I think this is probably to align with our team's style guidelines :)

yanksyoon · 2026-04-24T04:02:27Z

+class TestEnableLogForwarding(unittest.TestCase):
+    """Test suite for parsing, endpoint resolution, exporter detection, and config generation."""
+
+    def test_parse_files_into_list(self):


Could the tests have arrange/act/assert pattern in functional testing paradigm? This is to align with our team's style guidelines :)

yanksyoon · 2026-04-24T04:04:08Z

+SNAP_CMD = shutil.which("snap")
+SUDO_CMD = shutil.which("sudo")


I think we should hardcode the paths for these - so that the executables are not dynamically loaded, for security purposes.

yanksyoon · 2026-04-24T04:04:50Z

+        mkdir_result = run_as_root(
+            "mkdir", "-p", CONFIG_DIR  # create directory if it doesn't exist
+        )


Could this be using pathlib?

yanksyoon · 2026-04-24T04:07:25Z

+        install_result = run_as_root(
+            "install", "-m", "0644", tmp_path, config_path  # rw-r--r-- permissions
+        )


Should we prefer Pythonic way to install (copy and chmod) rather than calling install command?

florentianayuwono

review session from weii

florentianayuwono · 2026-04-24T13:09:23Z

@@ -0,0 +1,115 @@
+# Copyright 2026 Canonical Ltd.


for the folder name, we should use - instead of _

florentianayuwono · 2026-04-24T13:20:53Z

+      "start_at": "end"
+    }
+  },
+{{ exporters_section }}


do a test to see if it won't cause a conflict if the exporter is not optional and there are two config files has the same exporter

florentianayuwono · 2026-04-24T13:27:54Z

+def check_exporter_exists() -> bool:
+    """Return whether the configured exporter is already defined in collector config files."""
+    # Check for "  otlp_grpc:" exporter definition
+    pattern = f"^  {re.escape(EXPORTER_NAME)}:[ \t]*$"


currently this way of checking is not very reliable, it's either:

not needed, if there is no problem with same exporters exist in multiple config files

use a python yaml library to analyze the yaml file content (ex: PyYAML)

If the exporter is started, maybe there's a command line that dumps some information about the current configuration. If that's not started there's maybe a configuration validation command that can give information.

florentianayuwono · 2026-04-24T13:31:44Z

+        ("github.repository", os.getenv("GITHUB_REPOSITORY", "unknown")),
+        ("github.runner.name", os.getenv("RUNNER_NAME", "unknown")),
+        ("github.workflow", os.getenv("GITHUB_WORKFLOW", "unknown")),
+        ("github.job.id", os.getenv("GITHUB_JOB", "unknown")),


align the key name with https://github.com/canonical/github-runner-operator/pull/781/changes#diff-18077814ced5abb8b45d89d7940585ae954213dca8585ef4d48e693256da18e9 for the same ones

florentianayuwono · 2026-04-24T13:40:16Z

+    finally:
+        os.unlink(tmp_path)
+
+    logger.info("Wrote log-forwarding collector config to: %s", config_path)


log the final content of the config file, if wanna be fancy can use github action logging format https://docs.github.com/en/enterprise-server@3.18/actions/reference/workflows-and-actions/workflow-commands#grouping-log-lines

florentianayuwono · 2026-04-24T13:45:56Z

+        capture_output=True,
+        check=False,
+    )
+    if snap_list_result.returncode != 0:


in case it's not installed just install it

florentianayuwono · 2026-04-24T13:46:40Z

+## Prerequisites
+
+- Use a self-hosted Linux runner.
+- Install the `opentelemetry-collector` snap on the runner.


won't be needed if we install it for the runner in the action steps

florentianayuwono · 2026-04-24T13:48:43Z

+- Ensure the workflow can update `/etc/otelcol/config.d` with root privileges.
+
+## Provide inputs
+


ensure that you do not include any files that may contain sensitive information so that these info are not forwarded

erinecon

I approve the documentation changes 👍 since there's code-based changes in this PR, I didn't provide an official approval.

Spotted a couple more nits + responded to a Copilot comment

erinecon · 2026-04-24T13:49:36Z

I'm fine with this, would it be helpful if I opened an issue into the repo, or would you just prefer the Jira ticket?

Co-authored-by: Erin Conley <erin.conley@canonical.com>

gregory-schiano · 2026-04-27T08:38:03Z

+inputs:
+  files:
+    description: |
+      Newline or comma separated list of file paths or glob patterns to forward.


Nitpick, avoid double "or", so
Newline, comma separated list of file paths or

Reading the code, we support Comma separated, or list separated AND globbing. So you can have multiple globbing

gregory-schiano · 2026-04-27T08:38:30Z

+  files:
+    description: |
+      Newline or comma separated list of file paths or glob patterns to forward.
+      Example: /var/log/chrony/*.log


Provide an example for each supported content (newline, comma separated, and glob)

gregory-schiano · 2026-04-27T08:39:28Z

+  otlp-endpoint:
+    description: |
+      Optional OTLP/gRPC endpoint for upstream OpenTelemetry Collector logs export.
+      When not set, the action falls back to ACTION_OTEL_EXPORTER_OTLP_ENDPOINT.


Be more specific about where this env var comes from like:
injected automatically by self-hosted github runners provided in Canonical

gregory-schiano · 2026-04-27T08:40:51Z

+  config-file-name:
+    description: File name for the generated collector fragment.


Be specific about the usecase, renaming might helpful for priority mainly if they want this file to be evaluated before or after some other rules

gregory-schiano · 2026-04-27T08:46:23Z

bandit is missing
And a pyproject.toml needs to be added to configure the linting /mypy rules that we want (see for example https://github.com/canonical/wireguard-gateway-operator/blob/main/pyproject.toml)

gregory-schiano · 2026-04-27T08:56:39Z

+    """Parse comma/newline-separated file patterns into a normalized list."""
+    entries = []
+    # Split on commas or newlines, and strip whitespace. Ignore empty entries.
+    for item in re.split(r"[,\n]", files_input):


Put the regext as a constant

gregory-schiano · 2026-04-27T08:58:40Z

+    if not files:
+        logger.error("Input 'files' must contain at least one path or glob.")
+        sys.exit(1)


For coherence, in the other methods the guarding is done inside the private methods, so move this to parse_files_into_list

gregory-schiano · 2026-04-27T08:59:14Z

+
+def resolve_endpoint() -> str:
+    """Resolve OTLP endpoint from explicit input, then workflow fallback variable."""
+    # If INPUT_OTLP_ENDPOINT is not set, fall back to ACTION_OTEL_EXPORTER_OTLP_ENDPOINT


This can be in the docstring

gregory-schiano · 2026-04-27T09:01:49Z

+def check_exporter_exists() -> bool:
+    """Return whether the configured exporter is already defined in collector config files."""
+    # Check for "  otlp_grpc:" exporter definition
+    pattern = f"^  {re.escape(EXPORTER_NAME)}:[ \t]*$"


If the exporter is started, maybe there's a command line that dumps some information about the current configuration. If that's not started there's maybe a configuration validation command that can give information.

gregory-schiano · 2026-04-27T09:04:04Z

+
+    resolved_endpoint = resolve_endpoint()
+    exporter_already_exists = check_exporter_exists()
+    validate_exporter_configuration(resolved_endpoint, exporter_already_exists)


The logic can be moved here, the main method is not that big

Or you do the guarding in the build_config method

florentianayuwono and others added 9 commits April 22, 2026 22:53

feat: enable log forwarding through otel collector

ffd8772

chore(deps): update dependency packaging to v26.1 (#180)

14514f2

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

chore(deps): update dependency sphinx-ubuntu-images to v0.2.0 (#181)

77f12cf

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

chore(deps): replace astral-sh/setup-uv action with astral-sh/setup-u…

f41fd56

…v v8 (#182) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

fix(deps): update module github.com/jackc/pgx/v5 to v5.9.2 (#183)

11631ad

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

chore: update Copilot collections to v0.11.0 (#172)

564ff2c

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Christopher Bartz <christopher.bartz@canonical.com>

Merge branch 'main' into feat/log-forwarding-isd-5146

5961c47

florentianayuwono requested review from Copilot and erinecon April 22, 2026 15:59

Copilot started reviewing on behalf of florentianayuwono April 22, 2026 16:00 View session

fix: run shell as bash

1bdb99d

florentianayuwono marked this pull request as ready for review April 22, 2026 16:05

florentianayuwono requested review from cbartz, javierdelapuente, weiiwang01, yanksyoon and yhaliaw as code owners April 22, 2026 16:05

Copilot AI reviewed Apr 22, 2026

View reviewed changes

erinecon reviewed Apr 22, 2026

View reviewed changes

Comment thread docs/how-to/enable-log-forwarding.md

erinecon reviewed Apr 22, 2026

View reviewed changes

yanksyoon reviewed Apr 23, 2026

View reviewed changes

florentianayuwono and others added 4 commits April 23, 2026 10:21

Update docs/how-to/enable-log-forwarding.md

32eb82f

Co-authored-by: Erin Conley <erin.conley@canonical.com>

Apply suggestions from code review

f7f4147

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Erin Conley <erin.conley@canonical.com>

fix docs

0539529

add index

16a1ff7

cbartz reviewed Apr 23, 2026

View reviewed changes

yhaliaw reviewed Apr 23, 2026

View reviewed changes

Comment thread actions/enable-log-forwarding/enable-log-forwarding.py Outdated

florentianayuwono and others added 3 commits April 23, 2026 20:02

Update actions/enable-log-forwarding/enable-log-forwarding.py

9934f4e

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update actions/enable-log-forwarding/enable-log-forwarding.py

8337420

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'main' into feat/log-forwarding-isd-5146

9590fd5

erinecon reviewed Apr 23, 2026

View reviewed changes

Comment thread docs/how-to/enable-log-forwarding.md Outdated

florentianayuwono and others added 4 commits April 24, 2026 01:04

address code reviews

3f3fd59

Co-authored-by: Copilot <copilot@github.com>

add license

f15564a

Co-authored-by: Copilot <copilot@github.com>

fix docs

bc2a1e4

Co-authored-by: Copilot <copilot@github.com>

refactor code

5016dcb

Co-authored-by: Copilot <copilot@github.com>

florentianayuwono requested review from cbartz, Copilot, erinecon, yanksyoon and yhaliaw April 24, 2026 03:47

Copilot started reviewing on behalf of florentianayuwono April 24, 2026 03:47 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Comment thread docs/how-to/index.rst

yanksyoon approved these changes Apr 24, 2026

View reviewed changes

florentianayuwono commented Apr 24, 2026

View reviewed changes

erinecon reviewed Apr 24, 2026

View reviewed changes

Apply suggestions from code review

a920d9a

Co-authored-by: Erin Conley <erin.conley@canonical.com>

gregory-schiano reviewed Apr 27, 2026

View reviewed changes



		def build_config(files, resolved_endpoint, exporter_already_exists):
		attrs = [

		MODULE = load_module(MODULE_PATH)


		class TestEnableLogForwarding(unittest.TestCase):

		SNAP_CMD = shutil.which("snap")
		SUDO_CMD = shutil.which("sudo")

		- Ensure the workflow can update `/etc/otelcol/config.d` with root privileges.

		## Provide inputs

		config-file-name:
		description: File name for the generated collector fragment.

Conversation

florentianayuwono commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Why we need it

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erinecon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yanksyoon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

yanksyoon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

florentianayuwono commented Apr 22, 2026 •

edited

Loading